ci(automation): enhance repository automation and code quality workflows (#320) - #360
Conversation
…pache#355) - Create computer-rust crate with high-performance CSR graph representation, PageRank, SSSP, and atomic aggregator kernels - Implement C-ABI export layer (computer_rust_c_api.h) for FFI interoperability - Add dataset fixtures (Karate Club, synthetic power-law) and differential tolerance check suite - Add Java RustKernelBridge in computer-core with graceful fallback logic and unit tests - Add Go RustKernelBridge in vermeer with fallback execution and unit tests - Create .github/workflows/rust-ci.yml for Rust linting, testing, and formatting - Add docs/rust-modernization-roadmap.md detailing architecture, guardrails, baselines, and newcomer-friendly child tasks
…ows (apache#320) - Add spotless-maven-plugin to computer/pom.xml for automated Java code formatting (mvn spotless:apply / spotless:check) - Add jacoco-maven-plugin to computer/pom.xml for automated test coverage report generation - Create .github/workflows/commit-check.yml to validate PR titles against Conventional Commits formatting rules - Create .github/workflows/release-notes.yml for automated GitHub release draft generation - Create docs/automation-guide.md documenting repository code quality tools and PR guidelines
dosubot
Bot
added
size:XXL
imbajin
left a comment
imbajin
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: The exact head introduces correctness and native-integration blockers in the new Rust kernels, Java/Go bridges, and CI workflows. Evidence: static review of the exact base/head diff, plus exact-head workflow runs showing Rust CI startup_failure and other required checks in action_required.
| pub fn from_edges(num_vertices: u32, edges: &[(u32, u32, f64)]) -> Self { | ||
| let mut degree = vec![0; num_vertices as usize]; | ||
| for &(src, _dst, _weight) in edges { | ||
| if src < num_vertices { |
There was a problem hiding this comment.
num_vertices. For an edge such as (1, 99, 1.0), row_offsets reserves a slot that remains the default (target=0, weight=0), so PageRank and SSSP process a fabricated edge. Count only edges with both endpoints valid, or reject invalid endpoints at the API boundary.
| return -1; | ||
| } | ||
| let builder = unsafe { &mut *handle }; | ||
| builder.edges.push((src, dst, weight)); |
There was a problem hiding this comment.
computer_graph_add_edge accepts arbitrary f64 weights, but SsspKernel uses Dijkstra. Negative weights can return incorrect shortest paths and a reachable negative cycle can keep lowering distances and growing the heap; non-finite weights are also unbounded input. Reject non-finite and negative weights here, or change the algorithm and document the supported weight domain.
| use crate::RUST_KERNEL_VERSION; | ||
| use std::ffi::CString; | ||
| use std::os::raw::c_char; | ||
| use std::ptr; |
There was a problem hiding this comment.
std::ptr is unused. The added Rust workflow runs cargo clippy --all-targets -- -D warnings, so this import is promoted to an error and prevents the Rust CI job from reaching its tests. Remove the import or use it deliberately.
| return ranks; | ||
| } | ||
|
|
||
| private static native String nativeGetVersion(); |
There was a problem hiding this comment.
computer_kernel_version, but no JNI symbol for nativeGetVersion, and computePageRank never calls a native function. If the library loads, isAvailable() becomes true while version lookup falls back after UnsatisfiedLinkError and computation still runs in Java. Add JNI/C-ABI bindings for the required calls and make availability reflect callable symbols.
|
|
||
| func NewRustKernelBridge() *RustKernelBridge { | ||
| return &RustKernelBridge{ | ||
| available: false, |
There was a problem hiding this comment.
available is hard-coded to false, and this package contains no cgo declaration, library loading, or Rust C-ABI call. Vermeer therefore always executes the Go fallback even when the Rust library is deployed, making the new native bridge unreachable. Implement initialization and native calls, or remove the native-bridge claim and keep this as an explicitly fallback-only implementation.
| push: | ||
| branches: | ||
| - master | ||
| - /^release-.*$/ |
There was a problem hiding this comment.
^/release-.*$/ therefore does not match normal release-* branches, so Rust CI is skipped for release-branch pushes. Replace it with a supported glob such as release-* and verify the trigger.
| on: | ||
| push: | ||
| tags: | ||
| - 'v*' |
There was a problem hiding this comment.
v* tags, while the repository's existing release tags use the 1.0.0/1.5.0/1.7.0 form. A normal version tag will not run this workflow and will not produce draft release notes. Align the trigger with the repository's release convention or update the release process consistently.
2 participants
Description
This PR addresses the recommendations from the Automation Analysis report (#320) by introducing automatic code formatting, test coverage enforcement, PR commit validation, release notes generation, and developer automation documentation.
Key Changes
computer/pom.xml):spotless-maven-plugin(v2.30.0) for automated Java code formatting (mvn spotless:applyandmvn spotless:check).computer/pom.xml):jacoco-maven-plugin(v0.8.8) withprepare-agentandreportgoals to automatically generate test coverage reports duringmvn test..github/workflows/commit-check.yml):.github/workflows/release-notes.yml):v*) are pushed.docs/automation-guide.md):Reference
Fixes #320